Skip to content

WK4#4

Open
ivanmaker wants to merge 1 commit into
masterfrom
WK4
Open

WK4#4
ivanmaker wants to merge 1 commit into
masterfrom
WK4

Conversation

@ivanmaker

Copy link
Copy Markdown
Owner

I started re writing my program, I still haven't reached the testing phase yet.

I started re writing my program, I still haven't reached the testing phase yet.
@smarks

smarks commented Oct 5, 2018

Copy link
Copy Markdown

Please add circle ci part and I'll grade.

package StockServices;

import java.util.Calendar;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Javadoc

this.week = week;
}

public void weekChosen() {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure what this code is about?
you don't need this. Interval should be an enum the describes a period of time. e.g. a minute, hour, day etc.

StockServiceFactory stockServiceFactory = new StockServiceFactory();
basicStockService.getStockQuote(Symbol, From, Until, INTERVAL);
return stockServiceFactory;
/*returs stock service*/

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't need comments like this. what the code is doing is obvious

@Test
void getQuote4() {
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these aren't doing anything.

@smarks smarks left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please address comments and I'll re-grade. 

@smarks

smarks commented Oct 14, 2018

Copy link
Copy Markdown

have not see the fixes for this code yet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants